Add draw_tree visuals to tutorials 2&3#661
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
|
TODO:
|
|
@tturocy @rahulsavani Though I'm still fixing a couple of GH actions issues (see comment above), the actual tutorial updates are ready for review - see the "ReviewNB" button in the 2nd comment from top |
This reverts commit e3c58d6.
edwardchalstrey1
changed the title
| @@ -24,7 +24,9 @@ | |||
| "- If the Seller chooses **Honor**, both players receive payoffs of `1`;\n", | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
The intent here was to illustrate "the game is a root node, with no children" - it's not really necessary I could just remove this or revert to doing len(g.root.children)
There was a problem hiding this comment.
Ha, silly me, fine to keep it though if it could appear a bit to the right that would be better, but not problem if not.
| @@ -24,7 +24,9 @@ | |||
| "- If the Seller chooses **Honor**, both players receive payoffs of `1`;\n", | |||
There was a problem hiding this comment.
The edges for "Not" and "Trust" are very long and it looks a bit weird. Is that easy to change? I would imagine that this game would look fine if the angle of between the edges and the edge lengths at the root were the same or roughly the same as at the seller's node.
Reply via ReviewNB
| @@ -24,7 +24,9 @@ | |||
| "- If the Seller chooses **Honor**, both players receive payoffs of `1`;\n", | |||
There was a problem hiding this comment.
The four code blocks in this section have the code commented out. I guess that's unintentional?
Reply via ReviewNB
There was a problem hiding this comment.
This is intentional, I think getting the docs build to save and load files was a bit fiddly and someone running the code locally can run these by just un-commenting, what do you think? It could be possible
| @@ -30,7 +30,11 @@ | |||
| " - If he chooses to Call, he adds another $1 to the pot.\n", | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
Yep, I'll have a bit of a further fiddle with the default level spacing determining function in draw_tree (how it calculates based off the Gambit layout), though to note, the settings for this in draw_tree will ultimately be customisable parameters
rahulsavani
left a comment
rahulsavani
left a comment
There was a problem hiding this comment.
drawtree is such a great addition. Just a couple of minor comments in those two notebooks (one common to both: can we make the edges at the root node less long).
Fortest_tutorials.py, one minor comment:
- If I do
pytest --run-tutorials, all tests are run, not just for the tutorials; but - If I do
pytest tests/test_tutorials.py, the tutorials tests are skipped.
If it is easy to have a "run only tutorials" option that could be useful.
|
TODO:
|
…14 since open_spiel does not build correctly
edwardchalstrey1
changed the title
…e this is in active development
… version" This reverts commit 8b9307f.
|
Sorry @rahulsavani I just clicked re-review but the latest |
@rahulsavani Ok looks good now! |
edwardchalstrey1
moved this from In progress
to In review
in Ed Chalstrey Gambit priorities
|
Looks good to me! |
github-project-automation
bot
moved this from In review
to Done
in Ed Chalstrey Gambit priorities
2 participants
Adds
draw_treevisuals using the gambit layout to intro tutorials:Closes #552
Also includes changes from #668